Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

feat!: remove custom JWT decoding #3943

Merged
merged 6 commits into from
Apr 26, 2023
Merged

feat!: remove custom JWT decoding #3943

merged 6 commits into from
Apr 26, 2023

Conversation

robrap
Copy link
Contributor

@robrap robrap commented Apr 11, 2023 •
edited by zubair-ce07
Loading

Removes the ecommerce custom JWT decoding, and replaces with the simple decoding from the edx-drf-extensions library. Example replacements:

  • from ecommerce.extensions.api.handlers import jwt_decode_handler =>
  • from edx_rest_framework_extensions.auth.jwt.decoder import configured_jwt_decode_handler
    Or
  • 'ecommerce.extensions.api.handlers.jwt_decode_handler' =>
  • 'edx_rest_framework_extensions.auth.jwt.decoder.jwt_decode_handler'

Note that monitoring showed that all jwt_decode_handler calls were registering as ecom_jwt_decode_standard, and none as ecom_jwt_decode_custom, which shows that the custom code was no longer being used. The additional JWT_ISSUERS can now be cleaned up as a consequence of this change. Here is a private link for edx.org that can be used to confirm readiness for this change: https://onenr.io/0MR28Nrm2QY.

Other fixes in this PR:

  • Codecov was fixed to use the github action. It is not yet marked as required, because it was failing by too much.
  • Many constraints were dropped and many libraries were upgraded.
  • Some test improvements were required to work with the updated libraries.

Before merge:

Being reviewed under private ticket REV-3543.

⛔️ DEPRECATION WARNING

This repository is deprecated and in maintainence-only operation while we work on a replacement, please see this announcement for more information.

Although we have stopped integrating new contributions, we always appreciate security disclosures and patches sent to security@edx.org

Anyone internally merging to this repository is expected to release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.

Required Testing

  • Before deploying this change, complete a purchase in the stage environment.
    (^ We can remove that manual check once REV-2624 is done and the corresponding e2e test runs again)

Description

Describe what this pull request changes, and why these changes were made. How will these changes affect other people, installations of edx, etc.?
Please include links to any relevant ADRs, design artifacts, and decision documents. Make sure to document the rationale behind significant changes in the repo, per OEP-19, and can be
linked here.

Useful information to include:

  • Which edX user roles will this change impact? Common user roles are "Learner", "Course Author", "Developer", and "Operator".
  • Include screenshots for changes to the UI (ideally, both "before" and "after" screenshots, if applicable).
  • Provide links to the description of corresponding configuration changes. Remember to correctly annotate these changes.

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.

Testing instructions

Please provide detailed step-by-step instructions for testing this change; how did YOU test this change?

Other information

Include anything else that will help reviewers and consumers understand the change.

  • Does this change depend on other changes elsewhere?
  • Any special concerns or limitations? For example: deprecations, migrations, OpenEdx vs. edx.org differences, development vs. production environment differences, security, or accessibility.

@robrap robrap force-pushed the robrap/remove-custom-jwt-decoder branch 3 times, most recently from e5d7172 to 14d81fe Compare April 12, 2023 16:42
robrap
robrap commented Apr 12, 2023
View reviewed changes
requirements/base.txt
Copy link
Contributor Author

@robrap robrap Apr 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewer:

I used https://openedx.atlassian.net/wiki/spaces/AC/pages/1001521320/Python+Package+Changelogs to verify all changes that seemed like major version upgrades.

The change that required some updates (besides the JWT work) was around crispy_forms.

[request] Someone who knows, please spot check (or better) some views using crispy_forms.

The following are notes on all of the changes I verified:

bleach==5.0.1 => 6.0.0 (ok if tests pass)
cachetools==4.2.2 => 5.3.0 (requires Python 3.7+)
charset-normalizer==2.1.1 => 3.1.0 (upgrade probably handled by requests)
coverage==6.4.4 => 7.2.3 (? - code coverage should be looked at separately)
cryptography==38.0.1 => 40.0.1 (drops support for older OpenSSL and OSes)
datetime==4.7 => 5.1 (dropped older Python support)
django-crispy-forms==1.14.0 => 2.0 (Required some config changes. Could potentially cause UI issues with HTML in forms.)
django-filter==22.1 => 23.1 (no breaking changes)
edx-auth-backends==3.4.0 => 4.1.0 (JWT changes should be encapsulated to libraries)
edx-drf-extensions==6.6.0 => 8.5.3 (JWT changes should be encapsulated to libraries)
faker==14.2.0 => 18.4.0 (seems ok)
fixtures==3.0.0 => 4.0.1 (Dropped old Python support)
google-api-core==1.30.0 => 2.11.0 (Dropped old Python support)
google-auth==1.32.1 => 2.17.2 (Dropped old Python support)
multidict==5.1.0 => 6.0.4 (Dropped old Python support)
packaging==21.3 => 23.0 (Hoping this is ok; not sure what we package)
paramiko==2.11. => 3.1.0 (Should be ok, assuming other libraries have made fixes)
platformdirs==2.5.2 => 3.2.0 (Hopefully all works)
protobuf==3.17.3 => 4.22.1 (Hopefully all works)
pyjwt[crypto]==1.7.1 => 2.6.0 (JWT changes should be encapsulated to libraries)
pyopenssl==22.0.0 => 23.1.1 (No incompatible changes)
pyrsistent==0.18.1 => 0.19.3 (No incompatible changes)
pytz==2016.10 => 2023.3 (Seems ok)
requests-toolbelt==0.9.1 => 0.10.1 (No incompatible changes)
social-auth-app-django==4.0.0 => 5.2.0 (Drop old Python support)
stevedore==4.0.0 => 5.0.0 (Unknown, but should be ok)
stripe==4.1.0 => 5.4.0 (Dropped old Python; Excception typo fixes that don't affect us)
wheel==0.37.1 => 0.40.0 (Drop old Python support)
zope-interface==5.4.0 => 6.0 (Drop old Python support; other changes hopefully handled by dependencies)

robrap added 3 commits April 12, 2023 21:16
@robrap
feat!: remove custom JWT decoding
117d67d 
Removes the ecommerce custom JWT decoding, and replaces
with the simple decoding from the edx-drf-extensions
library.
@robrap
fix: drop constraints and make upgrade
7231120
@robrap
fix: handle major upgrade of django-crispy-forms
699a59f 
The major upgrade of django-crispy-forms called for
some changes related to bootstrap3 and dependencies.
See https://github.com/django-crispy-forms/django-crispy-forms/blob/main/CHANGELOG.md#major-changes-and-migration-guide
@robrap robrap force-pushed the robrap/remove-custom-jwt-decoder branch from 3d8f0dd to 04c086e Compare April 13, 2023 01:17
@robrap
fix: code coverage reporting
099ef7e 
Codecov no longer exists on PyPI, so switch to github
action to run coverage report.
@robrap robrap force-pushed the robrap/remove-custom-jwt-decoder branch from 04c086e to 099ef7e Compare April 13, 2023 01:41
@robrap
Copy link
Contributor Author

robrap commented Apr 13, 2023

Note to Reviewer: At this time, the only failing tests are codecov, which I plan to ignore since the data seems to be outdated.

@robrap robrap mentioned this pull request Apr 13, 2023
Open
@zubair-ce07 zubair-ce07 self-requested a review April 14, 2023 01:47
@robrap robrap mentioned this pull request Apr 20, 2023
Closed
zubair-ce07
zubair-ce07 approved these changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@zubair-ce07 zubair-ce07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@zubair-ce07 zubair-ce07 merged commit 679a1f7 into master Apr 26, 2023
@zubair-ce07 zubair-ce07 deleted the robrap/remove-custom-jwt-decoder branch April 26, 2023 06:13
grmartin added a commit that referenced this pull request Aug 29, 2023
@grmartin @adamstankiewicz
@jawad-khan @moeez96 @robrap @irfanuddinahmad @xitij2000 @aht007 @iloveagent57 @zubair-ce07 @iamsobanjaved @JadeyOlivier @saleem-latif @ishahroz @pshiu @brobro10000 @christopappas @UsamaSadiq
chore: Syncing 2u/project-theseus with master [2023-08-29] (#4022)
8a80bdf 
* build: Creating a missing workflow file `self-assign-issue.yml`.

The .github/workflows/self-assign-issue.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Creating a missing workflow file `add-remove-label-on-comment.yml`.

The .github/workflows/add-remove-label-on-comment.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* build: Updating a missing workflow file `add-depr-ticket-to-depr-board.yml`.

The .github/workflows/add-depr-ticket-to-depr-board.yml workflow is missing or needs an update to stay in
sync with the current standard for this workflow as defined in the
`.github` repo of the `openedx` GitHub org.

* docs: Remove repo specific CONTRIBUTING.rst

We now have a org wide CONTRIBUTING.md that points to our correct
general contributing guidelines.  We don't need repo specific ones that
forward to other contributing docs.

* fix: account for refunds in exec ed 2u redemption flow (#3920)

* chore: add logging to include fulfillment details upon GEAG allocation exception

* chore: quality

* fix: Pick the right purchase from ios response (#3921)

* fix: Pick the right purchase from ios response

iOS response contain multiple purchases, instead of picking the first purchase,
pick the one which have given product id and latest date.
LEARNER-9261

* feat: Added Android refund api (#3922)

* feat: Added Android refund api

Like Apple android doesn't have callback for every refund. Therefore we have created an endpoint  which we will hit daily through ecommerce worker.
Learner-9149

* feat: Error if products in basket are already purchased (#3929)

* feat: Error if products in basket are already purchased

* refactor: Add tests, Improve error message

* refactor: Update docstring

* test: Increase coverage

* chore: add logging to debug ent-6954 (#3931)

* fix: Fix error in checkout api for mobile (#3934)

* fix: Fix error in checkout api for mobile

* fix: Return error in case of duplicate transaction_id for mobile (#3936)

* fix: Return error in case of duplicate transactionID for mobile

* refactor: Review feedback, add documentation

* feat: Added course and expires field in product form on ecommerce dashboard (#3938)

Forked catalogue app from oscar and added course and expire field in ProductForm. This change will enable to
add Android sku from a same dashboard page.

* fix: reorder JWT decoders (#3941)

Reordered the JWT decoders to first use the standard
library version, and then use the custom ecommerce
decoder which uses multiple issuers. In this way, we
can see if any JWTs cannot be decoded by that standard
library version, and when and if we are ready to retire
the custom JWT decoding code.

See DEPR openedx/public-engineering#83

* fix: cached monitoring (#3942)

Monitoring features such as use of the increment
method, to increment a custom attribute, requires
the CachedCustomMonitoringMiddleware. This has been
added so the earlier calls to increment will function.

* feat: add discount_jwt monitoring (#3944)

Add monitoring for the discount JWT.

* feat: Added data_share_consent field to order fullfillment notes (#3939)

Co-authored-by: IrfanUddinAhmad <irfanahmad@arbisoft.com>

* chore: Switch from edx-sphinx-theme to sphinx-book-theme

The edx-sphinx theme is being deprecated, and replaced with sphinx-book-theme.
This removes references to the deprecated theme and replaces them with the new
standard theme for the platform.

* test: Add tests for Mobile IAP (#3937)

* test: Add tests for mobile In-app purchases

This reverts commit 54ea975.

* fix: fix codecov error

Codecov PyPI package was removed on 12 April and the recommended step is to migrate to codecov Github Action instead.

* fix: add an exec ex 2u max application check to the checkout flow. ENT-7059

Also removes codecov from ci.yml workflow.

* feat: add product entitlement info api (#3945)

* fix: Updated format for data_share_consent field

* docs: Update the contributing guidelines link.

We're moving towards a single set of guidelines org-wide.

* feat!: remove custom JWT decoding (#3943)

* feat!: remove custom JWT decoding

Removes the ecommerce custom JWT decoding, and replaces
with the simple decoding from the edx-drf-extensions
library.

* fix: drop constraints and make upgrade

* fix: handle major upgrade of django-crispy-forms

The major upgrade of django-crispy-forms called for
some changes related to bootstrap3 and dependencies.
See https://github.com/django-crispy-forms/django-crispy-forms/blob/main/CHANGELOG.md#major-changes-and-migration-guide

* fix: code coverage reporting

Codecov no longer exists on PyPI, so switch to github
action to run coverage report.

---------

Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>

* fix: Course to have multiple seats with certificate_type attribute (#3950)

* fix: Course to have multiple seats with certificate_type attribute
* refactor: Modify SKU generation hash, add tests
* test: Modify tests

* temp: update JWT_DECODE_HANDER in devstack.py

Jenkins job for building devstack images is temporarily broken.
This should fix the devstack settings until this configuration
change lands in an updated image:
openedx-unsupported/configuration#6921

* feat: add native Dockerfile to create ansible free image

* feat: add additional fields to EnterpriseLearnerOfferApiSerializer (#3963)

* refactor: add logging to mobile IAP (#3962)

* refactor: Improve exception handling for mobile IAP (#3969)

* refactor: Improve exception handling for mobile IAP
* refactor: pylint fixes

* feat: Fix capture_context error on Payment MFE (#3965)

* feat: Fix capture_context error on Payment MFE

* feat: removed whitespace

* feat: removed whitespace

* feat: modified test case

* feat: modified test case

---------

Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>

* feat: Add enterprise_customer_name in the event metadata for offer usage braze emails. (#3972)

* feat: Embargo check for subscription Programs (#3960)

* fix: Enable TrackingMiddleware for Mobile IAP basket (#3977)

* chore: updated Python requirements (edx-ecommerce-worker to version 3.3.3) (#3968)

Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>

* fix: schedule upgrade-python-requirements monthly

* fix: add edx-revenue-tasks to user_reviewers & remove team_reviewers

Will create a Jira ticket instead of tagging all the members of
@openedx/revenue-squad.

* feat: add sf line item field to enterprise offers

ENT-7013

* feat: Added ios refund callback (#3967)

* feat: add SDN endpoints (#3985)

* feat: add endpoint to run SDN check and return counts

* feat: add SDNCheckFailure REST APi

* fix: fix 500 on SDN for subscriptions (#3989)

* fix: fix 500 on SDN for subscriptions

* fix: pytest-selenium, pytest-variables, pyjwkest dependency issues (#3987)

* feat: add coupon sf opp line item attribute

* feat: Store price and currency for Mobile IAP (#3992)

* feat: Store price and currency for mobile IAP

* fix: return 200 on embargo failure to prevent downstream error (#3993)

* feat: Make mobile IAP execute/ API atomic (#3995)

* chore: added CODEOWNERS file (#3970)

* refactor: Add logging to mobile IAP checkout/ API (#4000)

* chore: django security patch 3.2.20 upgrade (#3999)

* feat: Updates opportunity line item regex and tests (#3996)

* feat: unenroll refunded android users daily (#4015)

* feat: unenroll refunded android users daily

Django management command to un-enroll refunded android users. This command will be run by Jenkins job daily.

* feat: mail mobile team for a mobile course change in publisher (#4014)

* feat: mail mobile team for a mobile course change in publisher

This will fix any unknown change from publisher to a course having mobile seats.
After this fix mobile team will see mail and adjust price of the course on playstore or appstore.
In the longer run we want to replace this solution by changing the course price directly using mobile platform apis.

LEARNER-9377

* fix: fixed coverage issue

---------

Co-authored-by: Feanil Patel <feanil@tcril.org>
Co-authored-by: Adam Stankiewicz <agstanki@gmail.com>
Co-authored-by: jawad khan <jawadkhan444@gmail.com>
Co-authored-by: Moeez Zahid <moeezzahid1996@gmail.com>
Co-authored-by: Robert Raposa <rraposa@edx.org>
Co-authored-by: irfanuddinahmad <34648393+irfanuddinahmad@users.noreply.github.com>
Co-authored-by: IrfanUddinAhmad <irfanahmad@arbisoft.com>
Co-authored-by: Kshitij Sobti <kshitij@sobti.in>
Co-authored-by: Mohammad Ahtasham ul Hassan <60315450+aht007@users.noreply.github.com>
Co-authored-by: Alex Dusenbery <adusenbery@edx.org>
Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>
Co-authored-by: Soban Javed <iamsobanjaved@gmail.com>
Co-authored-by: Jade Olivier <jadeolivier95@gmail.com>
Co-authored-by: Saleem Latif <saleem-latif@users.noreply.github.com>
Co-authored-by: Shahroz Ahmad <97090106+ishahroz@users.noreply.github.com>
Co-authored-by: Phillip Shiu <pshiu@users.noreply.github.com>
Co-authored-by: Phillip Shiu <pshiu@edx.org>
Co-authored-by: Hamzah Ullah <hamzahullah@yahoo.com>
Co-authored-by: jawad khan <jawad.khan@arbisoft.com>
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
Co-authored-by: Usama Sadiq <usama7274@gmail.com>
christopappas pushed a commit that referenced this pull request Dec 4, 2023
@robrap @zubair-ce07 @christopappas
feat!: remove custom JWT decoding (#3943)
8509752 
* feat!: remove custom JWT decoding

Removes the ecommerce custom JWT decoding, and replaces
with the simple decoding from the edx-drf-extensions
library.

* fix: drop constraints and make upgrade

* fix: handle major upgrade of django-crispy-forms

The major upgrade of django-crispy-forms called for
some changes related to bootstrap3 and dependencies.
See https://github.com/django-crispy-forms/django-crispy-forms/blob/main/CHANGELOG.md#major-changes-and-migration-guide

* fix: code coverage reporting

Codecov no longer exists on PyPI, so switch to github
action to run coverage report.

---------

Co-authored-by: Muhammad Zubair <syedzubairtahir12@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zubair-ce07 zubair-ce07 zubair-ce07 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@robrap @zubair-ce07